Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement packet_file_to_datasets function for CoDICE #804

Conversation

bourque
Copy link
Collaborator

@bourque bourque commented Aug 30, 2024

Change Summary

Overview

This PR implements the new utils.packet_file_to_datasets() function in the CoDICE pipelines.

Updated Files

  • imap_processing/codice/codice_l0.py
    • Moved XTCE file mapping to constants.py since it is used in multiple modules; implemented packet_file_to_datasets function
  • imap_processing/codice/codice_l1a.py
    • Implemented packet_file_to_datasets function and made a few other cleanup changes
  • imap_processing/codice/constants.py
  • Moved XTCE file mapping here since it is used in multiple modules now
  • imap_processing/tests/codice/test_codice_l0.py and imap_processing/tests/codice/test_codice_l1a.py
    • Updated tests to reflect new changes

Closes #771

…d "hard coded" into the configuration dictionary; Added spin sector config variable
…cessing pipeline a bit more readable; added proper numbers for positions/energies/spin_sectors
…arrays by positions, spin_sectors, and energies
…processing into codice-packet-file-to-dataset
@bourque bourque added Ins: CoDICE Related to the CoDICE instrument Level: L0 Level 0 processing Level: L1 Level 1 processing labels Aug 30, 2024
@bourque bourque requested review from joeymukherjee and a team August 30, 2024 21:06
@bourque bourque self-assigned this Aug 30, 2024
@bourque bourque requested review from greglucas and removed request for a team August 30, 2024 21:06
Comment on lines +178 to +183
(
1,
self.num_positions,
self.num_spin_sectors,
self.num_energy_steps,
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is to avoid 'unexpected argument' warnings from np.reshape()

@@ -309,49 +315,49 @@ def unpack_science_data(self, science_values: str) -> None:
science_values : str
A string of binary data representing the science values of the data.
"""
self.compression_algorithm = constants.LO_COMPRESSION_ID_LOOKUP[self.view_id]
compression_algorithm = constants.LO_COMPRESSION_ID_LOOKUP[self.view_id]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I originally wrote this, I thought that I might be using the compression_algorithm in various places in the module, but turns out I only need it here, so no need to make it a class attribute.

Copy link
Collaborator

@greglucas greglucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I think this looks great! A few minor comments is all.

Comment on lines +359 to +360
packet : xarray.Dataset
The packet to process.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

full_dataset ?

I think this is the entire packet file you are passing in and not an individual packet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I think this is just an individual packet? Here is how it is called:

    packets = packet_file_to_datasets(file_path, xtce_packet_definition)

    for apid in packets:
        packet = packets[apid]
        logger.info(f"\nProcessing {CODICEAPID(apid).name} packet")

        if apid == CODICEAPID.COD_NHK:
            dataset = create_hskp_dataset(packet, data_version)

        elif apid in [CODICEAPID.COD_LO_PHA, CODICEAPID.COD_HI_PHA]:
            dataset = create_event_dataset(apid, packet, data_version)

imap_processing/codice/codice_l1a.py Outdated Show resolved Hide resolved
epoch = xr.DataArray(
met_to_j2000ns(
metadata_arrays["SHCOARSE"],
packet.shcoarse.data,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should already be taken care of in the "epoch" variable. (I wonder if we will run into issues with you here and the different reference_epoch...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, good catch. So if I am understanding correctly, this can be changed from:

    epoch = xr.DataArray(
        met_to_j2000ns(
            packet.shcoarse.data,
            reference_epoch=np.datetime64("2010-01-01T00:01:06.184", "ns"),
        ),
        name="epoch",
        dims=["epoch"],
        attrs=cdf_attrs.get_variable_attributes("epoch"),
    )

to:

    epoch = xr.DataArray(
        packet.epoch,
        name="epoch",
        dims=["epoch"],
        attrs=cdf_attrs.get_variable_attributes("epoch"),
    )

imap_processing/codice/codice_l1a.py Show resolved Hide resolved
plan_id = packet.data["PLAN_ID"].raw_value
plan_step = packet.data["PLAN_STEP"].raw_value
view_id = packet.data["VIEW_ID"].raw_value
table_id = packet.table_id.data[0]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you able to remove any of the [0] indexes now and keep the entire array as you go through? I think we'd want each epoch associated with its own table_id and not just the first one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These four parameters are grabbed from a single APID/packet, so I think these will always be a single value. However, I can avoid the [0] by using int(packet.table_id.data)

imap_processing/codice/codice_l1a.py Show resolved Hide resolved
@@ -35,7 +35,7 @@
(1, 128), # lo-pha
]
EXPECTED_ARRAY_SIZES = [
123, # hskp
129, # hskp
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, you got a few more packets now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your comment here made me realize that the test for this is poorly named, because these values are actually the expected number of variables in the CDF (i.e. len(dataset)), not the data array sizes. I will update this in a new commit.

@bourque bourque merged commit d601a90 into IMAP-Science-Operations-Center:dev Sep 4, 2024
17 checks passed
@bourque bourque deleted the codice-packet-file-to-dataset branch September 4, 2024 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ins: CoDICE Related to the CoDICE instrument Level: L0 Level 0 processing Level: L1 Level 1 processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CoDICE: Implement new packet_file_to_datasets function in L1 pipeline
2 participants